Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mu beamspot constrained pT. Increase mu eta and phi precision to 16 bits #42646

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

namapane
Copy link
Contributor

PR description:

Following discussions between H4l and Hmumu analysis teams, we would like to put forward a proposal to add the beamspot-constrained muon pT to nanoAOD v13.

Some background:

A BS constraint applied on top of PAT:Muons has been already adopted for the mH measurement in 4l (HIG-21-019) after discussion/blessing in the muon POG [talk1]. It was shown to significantly improve the mH resolution, especially since 2018 when the BS widths are of the order of ~6 microns, and was preferred for such an high profile analysis to ad-hoc parametrizations (eg GeoFit) as it is a natural step in reconstruction rather than an heuristic a-posteriori correction, that requires tuning. There are also ongoing studies for using this for Hmumu.

At the time of discussion in the muon POG, including this in standard sequences was not considered, as this constrain can be easily applied by the user while running on miniAODs.

However, it is not possible to apply the constraint on top of NanoAODs, and this limitation is becoming apparent now. The statement that for mH4l we may be forced to stay with a miniAOD framework for this reason sparked some discussion within the ATTF framework surveys [talk2 - see last page of the discussion document at the end of the agenda]; there were several suggestions to investigate possible ways to include the required information in nanoAODs. My reply at the time was that the implementation of the "VX+BS" approach used in the H4l analysis includes both a BS constraint and a common vertex constraint among the 4 selected muons; this makes it impossible to define a general-purpose constrained pT variable.
But actually, for 2018 and Run3, the BS width is so small that essentially all improvement comes from the BS constraint, not the common vertex constraint, so constraining each muon individually would be enough.

The present implementation is based on SingleTrackVertexConstraint. It essentially just adds 3 variables: the updated pt, ptError (precision=6) and chi2 (precision=6) We checked that the variation of eta and phi (and of their errors) are negligible so it would not be necessary to store these.

One more small update included is the increase of the precision of Muon_eta and Muon_phi from 12 bits to 16 (code here). This is because with the current precision we have a small rounding error on the reconstructed invariant masses, of about ~3.5 MeV (RMS) for H4mu [talk3]. Although this is a very small effect from the statistical point of view, authors feel it is somewhat unsatisfactory for the world's most precise single-channel measurement of mH to add a rounding smearing after they been fighting to reduce the systematic down to 17 MeV (HIG-21-019, expected, 4mu only), especially if that can be avoided by just adding as little as 8 bits per muon.

PR validation:

Both changes have been tested on 2018 (using a backport) and 2022 data/MC.

@vlimant
Copy link
Contributor

vlimant commented Aug 23, 2023

enable nano

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42646/36700

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @namapane (Nicola Amapane) for master.

It involves the following packages:

  • PhysicsTools/NanoAOD (xpog)

@cmsbuild, @simonepigazzini, @vlimant can you please review it and eventually sign? Thanks.
@gpetruc, @AnnikaStein this is something you requested to watch as well.
@perrotta, @dpiparo, @antoniovilela, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@vlimant
Copy link
Contributor

vlimant commented Aug 23, 2023

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0bcfa/34453/summary.html
COMMIT: 875eb9d
CMSSW: CMSSW_13_3_X_2023-08-23-1100/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42646/34453/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 1 lines from the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 15 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3153095
  • DQMHistoTests: Total failures: 8
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3153065
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 28 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 15526
  • DQMHistoTests: Total failures: 42
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 15484
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 14 files compared)
  • Checked 31 log files, 14 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.0 2.454 2.447 0.008 ( +0.3% ) 5.11 5.13 -0.3% 2.156 2.115
2500.001 2.588 2.553 0.035 ( +1.4% ) 4.61 4.58 +0.6% 2.512 2.508
2500.002 2.507 2.498 0.010 ( +0.4% ) 4.73 4.76 -0.8% 2.502 2.498
2500.01 1.252 1.241 0.011 ( +0.9% ) 9.54 9.58 -0.3% 2.223 2.220
2500.011 1.619 1.605 0.015 ( +0.9% ) 5.15 5.20 -0.9% 2.418 2.423
2500.012 1.501 1.486 0.015 ( +1.0% ) 7.38 7.48 -1.3% 2.299 2.294
2500.1 2.169 2.161 0.009 ( +0.4% ) 5.15 5.19 -0.8% 2.013 1.936
2500.2 2.279 2.271 0.008 ( +0.4% ) 5.93 5.94 -0.1% 1.838 1.839
2500.21 1.161 1.147 0.014 ( +1.2% ) 4.31 4.32 -0.2% 2.224 2.230
2500.211 1.520 1.505 0.015 ( +1.0% ) 3.76 3.77 -0.5% 2.119 2.123
2500.3 2.018 2.009 0.009 ( +0.5% ) 12.00 12.22 -1.9% 1.872 1.711
2500.31 1.225 1.211 0.014 ( +1.2% ) 18.59 19.00 -2.2% 2.308 2.151
2500.311 1.604 1.590 0.015 ( +0.9% ) 13.05 14.13 -7.6% 2.181 2.010
2500.4 2.018 2.009 0.009 ( +0.5% ) 12.04 12.23 -1.5% 1.864 1.707

@vlimant
Copy link
Contributor

vlimant commented Aug 24, 2023

I wonder if MuonBeamspotConstraintValueMapProducer should be declared here or under some RecoMuon packages, given that it can also be ran at MINI level, I'd be in favour of moving it out of a NANO package
Can you please add a ddm plot for the new variables under https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/nanoDQM_cfi.py#L501

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42646/36710

@cmsbuild
Copy link
Contributor

Pull request #42646 was updated. @cmsbuild, @simonepigazzini, @vlimant can you please check and sign again.

@namapane
Copy link
Contributor Author

I wonder if MuonBeamspotConstraintValueMapProducer should be declared here or under some RecoMuon packages, given that it can also be ran at MINI level, I'd be in favour of moving it out of a NANO package

I have no strong opinion, but from a quick look I'd say that, in case, possible destinations could be RecoMuon/GlobalTrackingTools/plugins or RecoMuon/MuonIdentification/plugins. Both of these already include value map producers. Please let me know if I should move it.

Can you please add a ddm plot for the new variables under https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/nanoDQM_cfi.py#L501

Done.

@cmsbuild
Copy link
Contributor

Pull request #42646 was updated. @cmsbuild, @simonepigazzini, @mandrenguyen, @clacaputo, @vlimant can you please check and sign again.

@mandrenguyen
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0bcfa/34703/summary.html
COMMIT: 8367f57
CMSSW: CMSSW_13_3_X_2023-09-11-2300/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42646/34703/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 18 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 21 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3348561
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3348528
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 11.238000000000001 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 1.166 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 13234.0,... ): 0.769 KiB Physics/NanoAODDQM
  • Checked 214 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 28 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 15610
  • DQMHistoTests: Total failures: 42
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 15568
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 13.545000000000002 KiB( 14 files compared)
  • DQMHistoSizes: changed ( 2500.001,... ): 1.166 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.011,... ): 0.769 KiB Physics/NanoAODDQM
  • Checked 31 log files, 14 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.0 2.469 2.460 0.008 ( +0.3% ) 5.36 5.32 +0.9% 2.172 2.172
2500.001 2.611 2.596 0.015 ( +0.6% ) 4.77 4.81 -0.9% 2.604 2.606
2500.002 2.522 2.512 0.011 ( +0.4% ) 4.93 4.97 -0.9% 2.594 2.591
2500.01 1.264 1.253 0.011 ( +0.9% ) 10.00 9.60 +4.2% 2.294 2.301
2500.011 1.634 1.620 0.015 ( +0.9% ) 5.36 5.33 +0.5% 2.478 2.479
2500.012 1.517 1.502 0.014 ( +1.0% ) 7.65 7.69 -0.5% 2.387 2.396
2500.1 2.126 2.118 0.008 ( +0.4% ) 5.39 5.42 -0.6% 2.035 2.032
2500.2 2.237 2.229 0.008 ( +0.4% ) 6.17 6.21 -0.7% 1.948 1.945
2500.21 1.125 1.110 0.015 ( +1.3% ) 4.41 4.46 -1.1% 2.238 2.236
2500.211 1.479 1.464 0.015 ( +1.0% ) 3.92 3.87 +1.1% 2.321 2.317
2500.3 1.977 1.967 0.009 ( +0.5% ) 13.17 13.17 -0.0% 1.920 1.753
2500.31 1.190 1.175 0.015 ( +1.3% ) 20.96 21.26 -1.4% 2.319 2.149
2500.311 1.565 1.550 0.015 ( +1.0% ) 14.74 15.20 -3.0% 2.402 2.229
2500.4 1.977 1.967 0.009 ( +0.5% ) 13.17 13.07 +0.8% 1.922 1.749

@vlimant
Copy link
Contributor

vlimant commented Sep 12, 2023

spurious (unrelated) changes to eta/phi of muons, likely to be due to rounding effect

@vlimant
Copy link
Contributor

vlimant commented Sep 12, 2023

+1

@namapane
Copy link
Contributor Author

spurious (unrelated) changes to eta/phi of muons, likely to be due to rounding effect

That's actually intended: as mentioned in the PR description, the precision of muon eta/phi is increased to 16 bits instead of 12 (code here).

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen
Copy link
Contributor

type muon

@cmsbuild cmsbuild added the muon label Sep 13, 2023
@antoniovilela
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants